Skip to content

Optimize hash alloc #2512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dblock merged 11 commits into from
Nov 12, 2024
Merged

Optimize hash alloc #2512

dblock merged 11 commits into from
Nov 12, 2024

Conversation

@ericproulx
Copy link
Contributor

@ericproulx ericproulx commented Nov 11, 2024

This PR is all about hash allocation.

Double splat operator

Many functions had a **options or **_options where a simple options or options = {} can do the work.

Double splat is useful when keywords are passed explicitly or when you have keyword arguments. Something like

def my_function(**options)
  puts options
end

my_function(first_param: 'a', second_param: 'b')
# options = { first_param: 'a', second_param: 'b' }
def my_function(first_param:, **options)
  puts options
end

my_function(first_param: 'a', second_param: 'b')
# options = { second_param: 'b' }

Most of our internal functions are just passing a plain Hash that had to be ** because of the declared **options. The validator factory is a great example

module Grape
  module Validations
    class ValidatorFactory
      def self.create_validator(**options)
        options[:validator_class].new(options[:attributes],
                                      options[:options],
                                      options[:required],
                                      options[:params_scope],
                                      **options[:opts])
      end
    end
  end
end

the create_validator function is called like this

 yield Grape::Validations::ValidatorFactory.create_validator(**saved_validation)

and saved_validation is a plain Hash

Namespace

I found a @namespace_description in lib/grape/dsl/routing.rb#namespace that wasn't use at all so I removed it. It was creating some hashes.

Exceptions

  • Grape::Exceptions::Base had a **_options. I had to update Grape::Exceptions::Validation declaration and super usage to fit the expected parameters.

Validators

Grape:: Validations::Validators::Base had a *opts. It's been replaced by opts since its a static hash

Routing HEAD

I've optimized the mount_in function and add_head_not_allowed_methods_and_options_methods.

HEAD and GET are identical except that HEAD does not return content. Instead of creating a new Route for the HEAD, I've just dup the GET and replace the @request_method to HEAD

Benchmark

Before

Total allocated: 51146165 bytes (221499 objects)
Total retained:  5971658 bytes (15564 objects)

allocated memory by class
-----------------------------------
  27633240  Hash  <=====
   9604928  String
   6485704  Array
   4663405  Regexp
   2274584  ActiveSupport::OrderedOptions
    178224  Grape::Router::Route
    166896  Grape::Router::Pattern
     72720  Grape::Path
     30432  MatchData
     29400  Grape::Router::GreedyRoute
      1704  Class
      1224  Enumerator
       560  Proc
       488  Module
       448  Rack::Utils::HeaderHash


retained memory by class
-----------------------------------
   2274584  ActiveSupport::OrderedOptions
   1811845  Regexp
    933629  String
    305928  Array
    267800  Hash  <=====
    178224  Grape::Router::Route
    166896  Grape::Router::Pattern
     29400  Grape::Router::GreedyRoute
      1240  Class
       488  Module



Total allocated: 29384381 bytes (176924 objects)
Total retained:  5177218 bytes (12359 objects)

allocated memory by class
-----------------------------------
   9604928  String
   6699632  Hash  <=====
   6157528  Array
   4663405  Regexp
   1810584  ActiveSupport::OrderedOptions
    178224  Grape::Router::Route
    130896  Grape::Router::Pattern
     72720  Grape::Path
     30432  MatchData
     29400  Grape::Router::GreedyRoute


retained memory by class
-----------------------------------
   1811845  Regexp
   1810584  ActiveSupport::OrderedOptions
    933629  String
    178224  Grape::Router::Route
    144320  Hash  <=====
    134968  Array
    130896  Grape::Router::Pattern
     29400  Grape::Router::GreedyRoute
      1240  Class
       488  Module
       400  Proc
       280  Grape::Middleware::Stack::Middleware
       256  <<Unknown>>
       224  Rack::Utils::HeaderHash
       168  ActiveSupport::HashWithIndifferentAccess

@ericproulx
Remove double splat operators when not needed including specs
7862598 
Grape::Validations::Validators::Base last arguments is now just a simple param
Grape::Exceptions::Validation and Grape::Exceptions::ValidationErrors does not end with **args.
Grape::Request has a named param build_params_with: nil instead of **options
Remove @namespace_description in routing.rb
@ericproulx
Revert compile_many_routes.rb
d440674
@ericproulx
Optimize Head Route
d0da869
@ericproulx
Use dup instead of new for Head Route
6a20afb
@ericproulx
Use opts = {} for validator instead of just opts
42d93d4 
rubocop
@ericproulx
Optimize add_head_not_allowed_methods_and_options_methods
db7c156 
Renamed to add_head_and_options_methods
@ericproulx
Remove requirements and path from greedy route (not used)
ccbe23c 
allow header is joined
@ericproulx
Remove {} for opts since its internal.
d8a224c
@ericproulx
Fix rubocop
f3e08bf
@ericproulx
Remove frozen allow_header. Now Dynamic
bb7db5f
@ericproulx ericproulx marked this pull request as ready for review November 11, 2024 15:15
@ericproulx ericproulx requested a review from dblock November 11, 2024 15:16
dblock
dblock approved these changes Nov 12, 2024
View reviewed changes
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@dblock dblock merged commit 86648fa into ruby-grape:master Nov 12, 2024
56 checks passed
@ericproulx ericproulx deleted the optimize_hash_alloc branch November 16, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dblock dblock dblock approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ericproulx @dblock